Skip to content

Fix/template-duplicate-return-type #364

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Fix/template-duplicate-return-type #364

wants to merge 2 commits into from

Conversation

p1-ra
Copy link
Contributor

@p1-ra p1-ra commented Mar 26, 2021 •
edited
Loading

Title

Fix/template-duplicate-return-type

Description

Add a tiny fix on the endpoint template return_type macro, it ensure the unicity of the models' type.
When an endpoint use the same reference model for multiple responses (code), the model type was duplicated in function return types anotation.

Modified golden records diff before the fix:

diff --git a/end_to_end_tests/golden-record/my_test_api_client/api/tests/get_user_list.py b/end_to_end_tests/golden-record/my_test_api_client/api/tests/get_user_list.py
index ec22168..2e3a558 100644
--- a/end_to_end_tests/golden-record/my_test_api_client/api/tests/get_user_list.py
+++ b/end_to_end_tests/golden-record/my_test_api_client/api/tests/get_user_list.py
@@ -47,7 +47,9 @@ def _get_kwargs(
     }


-def _parse_response(*, response: httpx.Response) -> Optional[Union[List[AModel], HTTPValidationError]]:
+def _parse_response(
+    *, response: httpx.Response
+) -> Optional[Union[List[AModel], HTTPValidationError, HTTPValidationError]]:
     if response.status_code == 200:
         response_200 = []
         _response_200 = response.json()
@@ -61,10 +63,16 @@ def _parse_response(*, response: httpx.Response) -> Optional[Union[List[AModel],
         response_422 = HTTPValidationError.from_dict(response.json())

         return response_422
+    if response.status_code == 423:
+        response_423 = HTTPValidationError.from_dict(response.json())
+
+        return response_423
     return None
     
-def _build_response(*, response: httpx.Response) -> Response[Union[List[AModel], HTTPValidationError]]:
+def _build_response(
+    *, response: httpx.Response
+) -> Response[Union[List[AModel], HTTPValidationError, HTTPValidationError]]:
     return Response(
         status_code=response.status_code,
         content=response.content,
@@ -78,7 +86,7 @@ def sync_detailed(
     client: Client,
     an_enum_value: List[AnEnum],
     some_date: Union[datetime.date, datetime.datetime],
-) -> Response[Union[List[AModel], HTTPValidationError]]:
+) -> Response[Union[List[AModel], HTTPValidationError, HTTPValidationError]]:
     kwargs = _get_kwargs(
         client=client,
         an_enum_value=an_enum_value,
@@ -97,7 +105,7 @@ def sync(
     client: Client,
     an_enum_value: List[AnEnum],
     some_date: Union[datetime.date, datetime.datetime],
-) -> Optional[Union[List[AModel], HTTPValidationError]]:
+) -> Optional[Union[List[AModel], HTTPValidationError, HTTPValidationError]]:
     """ Get a list of things  """
          return sync_detailed(
@@ -112,7 +120,7 @@ async def asyncio_detailed(
     client: Client,
     an_enum_value: List[AnEnum],
     some_date: Union[datetime.date, datetime.datetime],
-) -> Response[Union[List[AModel], HTTPValidationError]]:
+) -> Response[Union[List[AModel], HTTPValidationError, HTTPValidationError]]:
     kwargs = _get_kwargs(
         client=client,
         an_enum_value=an_enum_value,
@@ -130,7 +138,7 @@ async def asyncio(
     client: Client,
     an_enum_value: List[AnEnum],
     some_date: Union[datetime.date, datetime.datetime],
-) -> Optional[Union[List[AModel], HTTPValidationError]]:
+) -> Optional[Union[List[AModel], HTTPValidationError, HTTPValidationError]]:
     """ Get a list of things  """

     return (

Modified golden records diff after the fix:

diff --git a/end_to_end_tests/golden-record/my_test_api_client/api/tests/get_user_list.py b/end_to_end_tests/golden-record/my_test_api_client/api/tests/get_user_list.py
index ec22168..2ed83e3 100644
--- a/end_to_end_tests/golden-record/my_test_api_client/api/tests/get_user_list.py
+++ b/end_to_end_tests/golden-record/my_test_api_client/api/tests/get_user_list.py
@@ -61,6 +61,10 @@ def _parse_response(*, response: httpx.Response) -> Optional[Union[List[AModel],
         response_422 = HTTPValidationError.from_dict(response.json())

         return response_422
+    if response.status_code == 423:
+        response_423 = HTTPValidationError.from_dict(response.json())
+
+        return response_423
     return None

@codecov
Copy link

codecov bot commented Mar 26, 2021 •
edited
Loading

Codecov Report

Merging #364 (6cd6d05) into main (95a29a8) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #364   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           47        47           
  Lines         1478      1478           
=========================================
  Hits          1478      1478

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 95a29a8...6cd6d05. Read the comment docs.

@dbanty
Copy link
Collaborator

dbanty commented Mar 27, 2021

Thanks @p1-ra! I created #365 which I think accomplishes the same thing with a bit less work. It moves the logic into Python so we don't have to do the join -> re-split thing (which was a cool workaround by the way). Take a look and make sure it's solving the same problem. If it is, I'd like to go that route.

@p1-ra
Copy link
Contributor Author

p1-ra commented Mar 30, 2021

It do solve the issue, more properly indeed 👍
I'm closing this issue. Thanks!

@p1-ra p1-ra closed this Mar 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@p1-ra @dbanty